Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not run apt update if skipRosdepInstall==true #942

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

roncapat
Copy link

@roncapat roncapat commented Dec 9, 2024

Closes #941

@roncapat roncapat requested a review from a team as a code owner December 9, 2024 19:42
@roncapat roncapat requested review from emersonknapp and MichaelOrlov and removed request for a team December 9, 2024 19:42
@christophebedard
Copy link
Member

christophebedard commented Dec 9, 2024

Wanting to skip apt update is reasonable. However, since it's currently not tied to the skip-rosdep-install option, I wonder if some people might be relying on action-ros-ci running apt update, e.g., to do something after action-ros-ci runs 🤔 just trying to figure out if this might break someone's workflow.

Signed-off-by: Patrick Roncagliolo <[email protected]>
@roncapat
Copy link
Author

Wanting to skip apt update is reasonable. However, since it's currently not tied to the skip-rosdep-install option, I wonder if some people might be relying on action-ros-ci running apt update, e.g., to do something after action-ros-ci runs 🤔 just trying to figure out if this might break someone's workflow.

@christophebedard any further thought on this? If you want I can add an additional boolean to make this backward-compatible in behaviour.

@christophebedard
Copy link
Member

I looked into it a bit more:

If someone is using setup-ros, that will take care of running apt update before action-ros-ci. They'd have to not use setup-ros (which would be the case when using a base Docker image, e.g., from setup-ros-docker), but use action-ros-ci and then try to use apt install. I think that should still work because the image should have some version of the apt repo package lists. Therefore, the risk of breaking someone's workflow should be pretty low; it's not worth adding a new option.

I would only ask you to mention this in the description for the skip-rosdep-install option:

Skip rosdep install.
. Something like:

diff --git a/action.yml b/action.yml
index 9e3631c..7409aab 100644
--- a/action.yml
+++ b/action.yml
@@ -81,7 +81,7 @@ inputs:
   skip-rosdep-install:
     default: ""
     description: |
-      Skip rosdep install.
+      Skip rosdep install. This also skips `apt update`/`dnf check-update`.
       Set to 'true'.
     required: false
   no-symlink-install:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow to skip APT update
2 participants